From c84bc16b351b87144c741ae3d07dae8b54f17b44 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 21 Nov 2014 12:31:00 -0800 Subject: [PATCH] Clean up code around the crawling of a directory The logic for "not recursing into `target`" was pretty hokey and needed replacement. This commit also unfies the paths a bit to ensure that the main loop is the same part that adds the root package itself. This reorganization ends up closing #937 --- src/cargo/core/package.rs | 9 +++ src/cargo/ops/cargo_read_manifest.rs | 71 ++++++++++-------------- src/cargo/ops/resolve.rs | 2 +- src/cargo/sources/path.rs | 2 +- src/rustversion.txt | 2 +- tests/support/mod.rs | 1 + tests/test_cargo_compile_custom_build.rs | 4 +- tests/test_cargo_package.rs | 39 ++++++++++++- 8 files changed, 82 insertions(+), 48 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index cdb57c6bf..594f685cb 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -1,4 +1,5 @@ use std::fmt::{mod, Show, Formatter}; +use std::hash; use std::slice; use semver::Version; @@ -128,6 +129,14 @@ impl PartialEq for Package { } } +impl Eq for Package {} + +impl hash::Hash for Package { + fn hash(&self, into: &mut hash::sip::SipState) { + self.get_package_id().hash(into) + } +} + #[deriving(PartialEq,Clone,Show)] pub struct PackageSet { packages: Vec, diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 955c15bcc..5d3a6f32d 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -27,20 +27,29 @@ pub fn read_package(path: &Path, source_id: &SourceId) pub fn read_packages(path: &Path, source_id: &SourceId) -> CargoResult> { - let mut all_packages = Vec::new(); + let mut all_packages = HashSet::new(); let mut visited = HashSet::::new(); log!(5, "looking for root package: {}, source_id={}", path.display(), source_id); - try!(process_possible_package(path, &mut all_packages, source_id, &mut visited)); - try!(walk(path, true, |root, dir| { + try!(walk(path, |dir| { log!(5, "looking for child package: {}", dir.display()); - if root && dir.join("target").is_dir() { return Ok(false); } - if root { return Ok(true) } + + // Don't recurse into git databases if dir.filename_str() == Some(".git") { return Ok(false); } - if dir.join(".git").exists() { return Ok(false); } - try!(process_possible_package(dir, &mut all_packages, source_id, + + // Don't automatically discover packages across git submodules + if dir != path && dir.join(".git").exists() { return Ok(false); } + + // Don't ever look at target directories + if dir.filename_str() == Some("target") && has_manifest(&dir.dir_path()) { + return Ok(false) + } + + if has_manifest(dir) { + try!(read_nested_packages(dir, &mut all_packages, source_id, &mut visited)); + } Ok(true) })); @@ -48,14 +57,14 @@ pub fn read_packages(path: &Path, Err(human(format!("Could not find Cargo.toml in `{}`", path.display()))) } else { log!(5, "all packages: {}", all_packages); - Ok(all_packages) + Ok(all_packages.into_iter().collect()) } } -fn walk(path: &Path, is_root: bool, - callback: |bool, &Path| -> CargoResult) -> CargoResult<()> { +fn walk(path: &Path, + callback: |&Path| -> CargoResult) -> CargoResult<()> { if path.is_dir() { - let continues = try!(callback(is_root, path)); + let continues = try!(callback(path)); if !continues { log!(5, "not processing {}", path.display()); return Ok(()); @@ -69,56 +78,36 @@ fn walk(path: &Path, is_root: bool, Err(e) => return Err(FromError::from_error(e)), }; for dir in dirs.iter() { - try!(walk(dir, false, |a, x| callback(a, x))) + try!(walk(dir, |x| callback(x))) } } Ok(()) } -fn process_possible_package(dir: &Path, - all_packages: &mut Vec, - source_id: &SourceId, - visited: &mut HashSet) -> CargoResult<()> { - - if !has_manifest(dir) { return Ok(()); } - - let packages = try!(read_nested_packages(dir, source_id, visited)); - push_all(all_packages, packages); - - Ok(()) -} - fn has_manifest(path: &Path) -> bool { find_project_manifest_exact(path, "Cargo.toml").is_ok() } -fn read_nested_packages(path: &Path, source_id: &SourceId, - visited: &mut HashSet) -> CargoResult> { - if !visited.insert(path.clone()) { return Ok(Vec::new()) } +fn read_nested_packages(path: &Path, + all_packages: &mut HashSet, + source_id: &SourceId, + visited: &mut HashSet) -> CargoResult<()> { + if !visited.insert(path.clone()) { return Ok(()) } let manifest = try!(find_project_manifest_exact(path, "Cargo.toml")); let (pkg, nested) = try!(read_package(&manifest, source_id)); - let mut ret = vec![pkg]; + all_packages.insert(pkg); // Registry sources are not allowed to have `path=` dependencies because // they're all translated to actual registry dependencies. if !source_id.is_registry() { for p in nested.iter() { - ret.extend(try!(read_nested_packages(&path.join(p), - source_id, - visited)).into_iter()); + try!(read_nested_packages(&path.join(p), all_packages, source_id, + visited)); } } - Ok(ret) -} - -fn push_all(set: &mut Vec, packages: Vec) { - for package in packages.into_iter() { - if set.contains(&package) { continue; } - - set.push(package) - } + Ok(()) } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 29947e693..e1bb3948d 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -96,7 +96,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, (d.get_name(), d) }).collect::>(); summary.map_dependencies(|d| { - match map.find_equiv(d.get_name()) { + match map.get(d.get_name()) { Some(&lock) if d.matches_id(lock) => d.lock_to(lock), _ => d, } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index af670de05..731e4835e 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -45,7 +45,7 @@ impl PathSource { return Err(internal("source has not been updated")) } - match self.packages.as_slice().head() { + match self.packages.iter().find(|p| p.get_root() == self.path) { Some(pkg) => Ok(pkg.clone()), None => Err(internal("no package found in source")) } diff --git a/src/rustversion.txt b/src/rustversion.txt index 9520ecf60..6bad8ad0d 100644 --- a/src/rustversion.txt +++ b/src/rustversion.txt @@ -1 +1 @@ -2014-11-22 +2014-11-24 diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 3c0de8f55..87a9caa00 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -517,3 +517,4 @@ pub static PACKAGING: &'static str = " Packaging"; pub static DOWNLOADING: &'static str = " Downloading"; pub static UPLOADING: &'static str = " Uploading"; pub static VERIFYING: &'static str = " Verifying"; +pub static ARCHIVING: &'static str = " Archiving"; diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index 93771ce63..9ea71f0db 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -241,8 +241,8 @@ test!(links_duplicates { native library `a` is being linked to by more than one package, and can only be \ linked to by one package - foo v0.5.0 (file://[..]) - a v0.5.0 (file://[..]) + [..] v0.5.0 (file://[..]) + [..] v0.5.0 (file://[..]) ")); }) diff --git a/tests/test_cargo_package.rs b/tests/test_cargo_package.rs index 434a39f02..57f2e64e9 100644 --- a/tests/test_cargo_package.rs +++ b/tests/test_cargo_package.rs @@ -2,9 +2,10 @@ use std::io::{File, MemReader}; use tar::Archive; use flate2::reader::GzDecoder; +use cargo::util::process; -use support::{project, execs, cargo_dir, ResultTest}; -use support::{PACKAGING, VERIFYING, COMPILING}; +use support::{project, execs, cargo_dir, ResultTest, paths, git}; +use support::{PACKAGING, VERIFYING, COMPILING, ARCHIVING}; use hamcrest::{assert_that, existing_file}; fn setup() { @@ -128,7 +129,41 @@ test!(metadata_warning { verifying = VERIFYING, compiling = COMPILING, dir = p.url()).as_slice())); +}) +test!(package_verbose { + let root = paths::root().join("all"); + let p = git::repo(&root) + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + "#) + .file("src/main.rs", r#" + fn main() {} + "#) + .file("a/Cargo.toml", r#" + [project] + name = "a" + version = "0.0.1" + authors = [] + "#) + .file("a/src/lib.rs", ""); + p.build(); + let cargo = process(cargo_dir().join("cargo")).unwrap() + .cwd(root) + .env("HOME", Some(paths::home())); + assert_that(cargo.clone().arg("build"), execs().with_status(0)); + assert_that(cargo.arg("package").arg("-v") + .arg("--no-verify"), + execs().with_status(0).with_stdout(format!("\ +{packaging} foo v0.0.1 ([..]) +{archiving} [..] +{archiving} [..] +", + packaging = PACKAGING, + archiving = ARCHIVING).as_slice())); }) test!(package_verification { -- 2.30.2